Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

migration: unlock/unreserve Gov v1 balances, remove Gov V1 pallets from polkadot runtime, and remove Gov V1 pallet key/values from storage #7314

Merged
merged 39 commits into from
Aug 18, 2023
Merged

migration: unlock/unreserve Gov v1 balances, remove Gov V1 pallets from polkadot runtime, and remove Gov V1 pallet key/values from storage #7314

merged 39 commits into from
Aug 18, 2023

Conversation

liamaharon
Copy link
Contributor

@liamaharon liamaharon commented May 31, 2023 •
edited
Loading

Partial paritytech/polkadot-sdk#485

Checklist

Migration summary

Network Module Total Accounts Total Stake to Unlock Total Deposit to Unreserve USD Total
Kusama Democracy 5364 1,179,551 KSM 25 KSM $30,609,997
Kusama ElectionPhragmen 748 1,496,706 KSM 157 KSM $38,843,594
Kusama Tip 3 N/A 0.21 KSM $5
Polkadot Democracy 5228 44,984,449 DOT 800 DOT $260,914,444
Polkadot ElectionPhragmen 326 235,329,403 DOT 5,649 DOT $1,364,943,301
Polkadot Tip 7 N/A 16 DOT $92

assuming DOT: $5.8, KSM: $25.95

Questions / to figure out

  • Total to unstake amounts seem quite high, but not necessarily invalid. Are there any dashboards we can cross-check them against?~
  • What's the best way to disable calls to Gov V1 pallets, incl. if they're made as part of batch requests?
  • ERROR main runtime::system: Logic error: Unexpected underflow in reducing consumer error logs

@liamaharon liamaharon added A3-in_progress Pull request is in progress. No review needed at this stage. C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit. B1-note_worthy Changes should be noted in the release notes labels May 31, 2023
@kianenigma kianenigma mentioned this pull request May 31, 2023
Closed
12 tasks
@liamaharon liamaharon mentioned this pull request May 31, 2023
Merged
1 task
@liamaharon liamaharon added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels May 31, 2023
@liamaharon liamaharon marked this pull request as ready for review May 31, 2023 10:30
@paritytech-ci paritytech-ci requested review from a team May 31, 2023 10:30
@liamaharon liamaharon added the T1-runtime This PR/Issue is related to the topic “runtime”. label May 31, 2023
@paritytech-ci paritytech-ci requested a review from a team June 13, 2023 12:37
@ggwpez ggwpez added the E0-runtime_migration PR introduces code that might require downstream chains to run a runtime upgrade. label Jun 13, 2023
@liamaharon liamaharon requested a review from kianenigma June 13, 2023 16:15
@ggwpez
Copy link
Member

ggwpez commented Jun 13, 2023 •
edited
Loading

What is this in the Kusama try-runtime log? Looks like a few hundreds of those.

ERROR main runtime::system: Logic error: Unexpected underflow in reducing consumer

@liamaharon
Copy link
Contributor Author

liamaharon commented Jun 13, 2023 •
edited
Loading

What is this in the Kusama try-runtime log? Looks like a few hundreds of those.

ERROR main runtime::system: Logic error: Unexpected underflow in reducing consumer

It occurs when a lock is removed on an account that has zero consumers set, it is benign and can safely be ignored.

@gpestana suggested in this comment that it's likely to be related to paritytech/polkadot-sdk#234.

I was unable to log the offending accounts to check their state exactly (see the comment), but I did confirm that all the accounts "Unexpected underflow in reducing consumer" occurs for have real locks in Balances, so it's necessary to T::Currency::remove_lock on those accounts even if it triggers that log, otherwise it'll leave hanging locks in the Balances.

ggwpez
ggwpez reviewed Jun 14, 2023
View reviewed changes
node/service/src/chain_spec.rs Outdated Show resolved Hide resolved
runtime/kusama/src/governance/old.rs Outdated Show resolved Hide resolved
runtime/kusama/src/lib.rs Outdated Show resolved Hide resolved
runtime/kusama/src/lib.rs Show resolved Hide resolved
runtime/kusama/src/lib.rs Outdated Show resolved Hide resolved
runtime/kusama/src/lib.rs Outdated Show resolved Hide resolved
@liamaharon
Copy link
Contributor Author

Needs paritytech/substrate#14779. Then, should be no more blockers 🤞

bkchr
bkchr approved these changes Aug 17, 2023
View reviewed changes
Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ty! Much better approach! :)

@liamaharon liamaharon changed the title migration: unlock/unreserve Gov v1 balances and prepare storage removal migration: unlock/unreserve Gov v1 balances and remove kvs Aug 18, 2023
liamaharon
liamaharon commented Aug 18, 2023
View reviewed changes
runtime/polkadot/src/lib.rs
Comment on lines -205 to -209
// Check which one has more yes votes.
(
OriginCaller::Council(pallet_collective::RawOrigin::Members(l_yes_votes, l_count)),
OriginCaller::Council(pallet_collective::RawOrigin::Members(r_yes_votes, r_count)),
) => Some((l_yes_votes * r_count).cmp(&(r_yes_votes * l_count))),
Copy link
Contributor Author

@liamaharon liamaharon Aug 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mirroring Kusama change: https://github.com/paritytech/polkadot/pull/6701/files#diff-db5840c3c03c740de25aed56b9257559c224a04619c079c94cddbb23f9a855a4L210-L214

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: test-linux-stable
Logs: https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/3410819

@liamaharon
Copy link
Contributor Author

bot merge force

@paritytech-processbot paritytech-processbot bot merged commit a877cc5 into master Aug 18, 2023
@paritytech-processbot paritytech-processbot bot deleted the liam-kusama-gov-v1-storage-migration branch August 18, 2023 09:03
@liamaharon liamaharon changed the title migration: unlock/unreserve Gov v1 balances and remove kvs migration: unlock/unreserve Gov v1 balances, remove Gov V1 pallets from polkadot runtime, and remove Gov V1 pallet key/values from storage Aug 18, 2023
@liamaharon liamaharon mentioned this pull request Sep 11, 2023
Merged
@liamaharon liamaharon mentioned this pull request Sep 18, 2023
Closed
@al3mart al3mart mentioned this pull request Oct 11, 2023
Merged
@crystalin crystalin mentioned this pull request Oct 20, 2023
Closed
@JesseAbram JesseAbram mentioned this pull request Oct 26, 2023
Merged
@liamaharon liamaharon mentioned this pull request Nov 13, 2023
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@gupnik gupnik gupnik left review comments

@kianenigma kianenigma kianenigma approved these changes

@ggwpez ggwpez ggwpez approved these changes

@bkchr bkchr bkchr approved these changes

@rphmeier rphmeier Awaiting requested review from rphmeier

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review. B1-note_worthy Changes should be noted in the release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit. E0-runtime_migration PR introduces code that might require downstream chains to run a runtime upgrade. T1-runtime This PR/Issue is related to the topic “runtime”.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@liamaharon @ggwpez @paradox-tt @Polkadot-Forum @thekiwimango @paritytech-cicd-pr @kianenigma @bkchr @rphmeier @gupnik